updated to latest algebra #994

kailuowang · 2016-04-24T12:07:55Z

No description provided.

non · 2016-04-24T12:29:01Z

This PR looks fine to me. 👍 if we can get Travis to pass (not sure why it failed)

kailuowang · 2016-04-24T13:25:09Z

@non looks like that https://codecov.io/bash is down, causing the second part of the following command to fail

$sbt_cmd coverage validateJVM coverageReport && bash <(curl -s https://codecov.io/bash)"

update: turns out codecov is taking a maintenance break. http://status.codecov.io/

non · 2016-04-24T13:30:44Z

OK, great. We can just hang out and wait for that to finish. Thanks for investigating!

kailuowang · 2016-04-24T16:05:38Z

Looks like codecov is back online. Can we restart the build?

non · 2016-04-24T16:08:58Z

Yeah let's try it again.

kailuowang · 2016-04-24T16:37:23Z

Looks like that this time it failed on (or after) uploading coverage report to S3. Maybe codecov is still having issues.

adelbertc · 2016-04-24T17:59:47Z

laws/src/main/scala/cats/laws/discipline/Eq.scala

-      def eqv(x: (A, B), y: (A, B)): Boolean =
-        A.eqv(x._1, y._1) && B.eqv(x._2, y._2)
-    }
+  implicit def tuple2Eq[A, B](implicit A: Eq[A], B: Eq[B]): Eq[(A, B)] = tuple.tuple2Eq


We should be able to just remove these right? And do the correct import at the places that use this?

I admit I was too lazy. Just updated.

adelbertc · 2016-04-24T18:03:38Z

Can you also add extend the appropriate tuple instances trait like we do /~https://github.com/typelevel/cats/blob/master/core/src/main/scala/cats/std/anyval.scala#L18 so users only need 1 import?

kailuowang · 2016-04-24T20:48:51Z

@adelbertc IRT your last comment about adding the algebra tuple to cats.std, the bad news is that the generated algebra.std.TupleInstances is package private. Do you want to open it up and release algebra 0.4.1 or shall we have this wait for the next cats release?

non · 2016-04-24T20:49:43Z

@kailuowang let's open that up -- we just released 0.4.0 and i think that is pretty important

kailuowang · 2016-04-24T21:02:38Z

@non makes sense. I will send in a PR to algebra soon.

adelbertc · 2016-04-24T22:48:15Z

typelevel/algebra#151

ceedubs · 2016-04-25T13:21:17Z

The build issues should be fixed. I'm going to try to close and reopen this to see if it makes Travis CI happy.

kailuowang · 2016-04-25T13:25:11Z

Don't merge yet, waiting on a new algebra release so we can address @adelbertc's last comment

ceedubs · 2016-04-25T13:34:21Z

Do we have an issue with scala.js versions of algebra? https://travis-ci.org/typelevel/cats/jobs/125562475#L3310-L3313

kailuowang · 2016-04-25T17:14:37Z

@ceedubs, that's due to the fact that I forgot to update the algebra.law version down in the build.sbt. just pushed in a fix.

non · 2016-04-25T17:48:03Z

Looks good to me. 👍

adelbertc · 2016-04-25T18:23:50Z

tests/src/test/scala/cats/tests/StateTTests.scala

@@ -5,6 +5,7 @@ import cats.laws.discipline.{CartesianTests, MonadStateTests, SerializableTests}
 import cats.data.{State, StateT}
 import cats.laws.discipline.eq._
 import cats.laws.discipline.arbitrary._
+import cats.std.tuple._


Is this needed?

adelbertc · 2016-04-25T18:24:03Z

👍 from me, modulo 2 nitpicky comments

adelbertc · 2016-04-25T18:25:12Z

core/src/main/scala/cats/data/WriterT.scala

-  // on an algebra release that includes /~https://github.com/non/algebra/pull/82
-  implicit def writerTIdEq[L, V](implicit E: Eq[(L, V)]): Eq[WriterT[Id, L, V]] =
+  implicit def writerTIdEq[L: Eq, V: Eq]: Eq[WriterT[Id, L, V]] = {
+    import algebra.std.tuple.tuple2Eq


Can we move this up top with the rest of the imports?

kailuowang · 2016-04-25T18:30:48Z

@adelbertc's comments addressed.

travisbrown · 2016-04-25T18:34:35Z

docs/src/main/tut/monoid.md

@@ -60,23 +60,9 @@ l.foldMap(i => i.toString)
 ```

 To use this
-with a function that produces a tuple, we can define a `Monoid` for a tuple 
+with a function that produces a tuple, cats also provides a `Monoid` for a tuple


Is this potentially confusing since these actually come from Algebra and the following sentence goes into detail about the role Algebra plays here?

I used cats here because it is available through cats.std. and I didn't notice the sentence below. I will add the detail about the instance below as well.

update: N.B. added below. Please help review the wording.

codecov-io · 2016-04-25T19:32:10Z

Current coverage is 83.76%

Merging #994 into master will decrease coverage by -0.00%

@@             master       #994   diff @@
==========================================
  Files           368        368          
  Lines          2367       2365     -2   
  Methods        2162       2157     -5   
  Messages          0          0          
  Branches         16         20     +4   
==========================================
- Hits           1983       1981     -2   
  Misses          384        384          
  Partials          0          0

4 files (not in diff) in .../main/scala/cats/std were modified. more
2 files (not in diff) in ...n/scala/cats/functor were modified. more
2 files (not in diff) in ...main/scala/cats/data were modified. more
- Hits +2
11 files (not in diff) in ...cats/laws/discipline were modified. more
25 files (not in diff) in ...in/scala/cats/syntax were modified. more
7 files (not in diff) in .../main/scala/cats/std were modified. more
3 files (not in diff) in ...n/scala/cats/functor were modified. more
3 files (not in diff) in ...main/scala/cats/free were modified. more
2 files (not in diff) in ...main/scala/cats/data were modified. more
1 files (not in diff) in .../src/main/scala/cats were modified. more

Powered by Codecov. Last updated by 7cdbd34

adelbertc · 2016-04-25T19:35:23Z

👍 - if @travisbrown is OK with the comment changes let's merge

ceedubs · 2016-04-26T11:18:07Z

👍 the clarification about algebra tuple instances looks good to me. I'm going to go ahead and merge. @travisbrown if you have suggestions for further improvements it should be easy for us to make a doc update after merging this.

travisbrown · 2016-04-26T12:01:44Z

Sorry for the delay, but yes, +1 from me.

adelbertc reviewed Apr 24, 2016
View reviewed changes

kailuowang force-pushed the update-algebra branch from 654257a to 18b3912 Compare April 24, 2016 20:43

ceedubs closed this Apr 25, 2016

ceedubs reopened this Apr 25, 2016

ceedubs added the in progress label Apr 25, 2016

kailuowang force-pushed the update-algebra branch from 18b3912 to 346e4c1 Compare April 25, 2016 15:22

kailuowang changed the title ~~updated to Algebra 0.4.0~~ updated to latest algebra Apr 25, 2016

kailuowang force-pushed the update-algebra branch 3 times, most recently from ffe0c64 to 8bbdd48 Compare April 25, 2016 17:14

non mentioned this pull request Apr 25, 2016

Introduce cats-kernel and remove algebra dependency #1001

Merged

adelbertc reviewed Apr 25, 2016
View reviewed changes

kailuowang force-pushed the update-algebra branch from 8bbdd48 to a8da9e4 Compare April 25, 2016 18:30

kailuowang closed this Apr 25, 2016

stew removed the in progress label Apr 25, 2016

kailuowang reopened this Apr 25, 2016

kailuowang force-pushed the update-algebra branch from a8da9e4 to c18cdbb Compare April 25, 2016 18:32

travisbrown reviewed Apr 25, 2016
View reviewed changes

updated to algebra 0.4.2

36c5a33

kailuowang force-pushed the update-algebra branch from c18cdbb to 36c5a33 Compare April 25, 2016 18:54

ceedubs merged commit b679435 into typelevel:master Apr 26, 2016

kailuowang deleted the update-algebra branch April 26, 2016 11:48

travisbrown mentioned this pull request May 14, 2016

Let's talk the next release (0.5.0) #966

Closed

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

updated to latest algebra #994

updated to latest algebra #994

kailuowang commented Apr 24, 2016

non commented Apr 24, 2016

kailuowang commented Apr 24, 2016 •

edited

Loading

non commented Apr 24, 2016

kailuowang commented Apr 24, 2016

non commented Apr 24, 2016

kailuowang commented Apr 24, 2016

adelbertc Apr 24, 2016

kailuowang Apr 24, 2016

adelbertc commented Apr 24, 2016

kailuowang commented Apr 24, 2016

non commented Apr 24, 2016

kailuowang commented Apr 24, 2016 •

edited

Loading

adelbertc commented Apr 24, 2016

ceedubs commented Apr 25, 2016

kailuowang commented Apr 25, 2016

ceedubs commented Apr 25, 2016

kailuowang commented Apr 25, 2016

non commented Apr 25, 2016

adelbertc Apr 25, 2016

adelbertc commented Apr 25, 2016 •

edited

Loading

adelbertc Apr 25, 2016

kailuowang commented Apr 25, 2016

travisbrown Apr 25, 2016

kailuowang Apr 25, 2016

kailuowang Apr 25, 2016

codecov-io commented Apr 25, 2016

adelbertc commented Apr 25, 2016 •

edited

Loading

ceedubs commented Apr 26, 2016

travisbrown commented Apr 26, 2016

updated to latest algebra #994

updated to latest algebra #994

Conversation

kailuowang commented Apr 24, 2016

non commented Apr 24, 2016

kailuowang commented Apr 24, 2016 • edited Loading

non commented Apr 24, 2016

kailuowang commented Apr 24, 2016

non commented Apr 24, 2016

kailuowang commented Apr 24, 2016

adelbertc Apr 24, 2016

Choose a reason for hiding this comment

kailuowang Apr 24, 2016

Choose a reason for hiding this comment

adelbertc commented Apr 24, 2016

kailuowang commented Apr 24, 2016

non commented Apr 24, 2016

kailuowang commented Apr 24, 2016 • edited Loading

adelbertc commented Apr 24, 2016

ceedubs commented Apr 25, 2016

kailuowang commented Apr 25, 2016

ceedubs commented Apr 25, 2016

kailuowang commented Apr 25, 2016

non commented Apr 25, 2016

adelbertc Apr 25, 2016

Choose a reason for hiding this comment

adelbertc commented Apr 25, 2016 • edited Loading

adelbertc Apr 25, 2016

Choose a reason for hiding this comment

kailuowang commented Apr 25, 2016

travisbrown Apr 25, 2016

Choose a reason for hiding this comment

kailuowang Apr 25, 2016

Choose a reason for hiding this comment

kailuowang Apr 25, 2016

Choose a reason for hiding this comment

codecov-io commented Apr 25, 2016

Current coverage is 83.76%

adelbertc commented Apr 25, 2016 • edited Loading

ceedubs commented Apr 26, 2016

travisbrown commented Apr 26, 2016

kailuowang commented Apr 24, 2016 •

edited

Loading

kailuowang commented Apr 24, 2016 •

edited

Loading

adelbertc commented Apr 25, 2016 •

edited

Loading

adelbertc commented Apr 25, 2016 •

edited

Loading